Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Issue #3322] Transform opportunity attachment files #3486

Merged
merged 19 commits into from
Jan 17, 2025

Conversation

chouinar
Copy link
Collaborator

@chouinar chouinar commented Jan 10, 2025

Summary

Fixes #3322

Time to review: 10 mins

Changes proposed

A lot of file utilities (used in this PR) for handling reading/writing/naming files on s3

Utility for setting up s3 file paths for the attachments

Logic to handle inserts/updates/deletes of attachments and the files that need to move around on s3.

Context for reviewers

There are some scenarios I haven't accounted for yet when the opportunity itself is modified (deleted / is no longer a draft), I originally wanted to handle this in a single PR, but I'll split that out as this one already was getting too big.

See the ticket for details on the scenarios we need to handle.

Additional information

Testing this is a bit tedious - there is a lot that needs to be setup exactly to test it.

I'd recommend nuking anything you already have with make volume-recreate

Set the env var to enable the job to run (add TRANSFORM_ORACLE_DATA_ENABLE_OPPORTUNITY_ATTACHMENT=1 to override.env)

Run make console and in that do f.StagingTsynopsisAttachmentFactory.create_batch(size=50) and then exit()

Finally you can run the job by doing make cmd args="data-migration load-transform --no-load --transform --no-set-current"

@chouinar chouinar requested a review from mdragon as a code owner January 10, 2025 17:58
Base automatically changed from chouinar/3271-attachment-transform to main January 15, 2025 19:00
api/tests/conftest.py Outdated Show resolved Hide resolved
@babebe
Copy link
Collaborator

babebe commented Jan 15, 2025

Run both locally and tests LGTM!!! Great util functions!

babebe
babebe previously approved these changes Jan 15, 2025
chouinar and others added 2 commits January 16, 2025 12:19
…ublished (#3503)

## Summary
Fixes #3500

### Time to review: __10 mins__

## Changes proposed
Handle the following two cases during the transformation process
* Opportunity deleted - clean up attachments from s3
* Opportunity stops being a draft - move all the attachments to the
other s3 bucket

## Context for reviewers
Mostly just some additional file utils for moving files to handle the
above scenarios. Only noteworthy callout is that there isn't really a
concept of "moving" a file on s3, it's just a copy+delete.
Copy link
Collaborator

@babebe babebe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@mdragon mdragon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@chouinar chouinar merged commit b4ccc44 into main Jan 17, 2025
2 checks passed
@chouinar chouinar deleted the chouinar/file-attachment-transform branch January 17, 2025 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement the transformation for attachment files
3 participants